Skip to content

Conversation

@nan-li
Copy link
Contributor

@nan-li nan-li commented Nov 20, 2025

Description

One Line Summary

Add basic infrastructure to test the OneSignalInAppMessages module, and begin migrating tests.

Details

  • We want to test the Objective-C framework OneSignalInAppMessages using Swift language, primarily.
  • Bridging headers allow for exposing Objective-C headers to Swift, which are used in the migrated tests.

Motivation

Increase tests, porting over tests.

Scope

Testing only, not changing the shipped SDK, or making any internal APIs public.

Testing

Unit testing

  • Add Swift lang test file IAMIntegrationTests and migrate 2 integration tests
    • In attempt to migrate existing testDisablingIAMs_stillCreatesMessageQueue_butPreventsMessageDisplay to new tests, but behavior has changed. It becomes testPausingIAMs_doesNotCreateMessageQueue.
  • Add new module OneSignalInAppMessagesMocks for re-usable test helpers and mock functionality
  • Add ConsistencyManagerTestHelpers with methods for repeating functionality

Manual testing

  • Just the app builds and runs still.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes
  • Tests

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

This change is Reviewable

@nan-li nan-li added the WIP Work In Progress label Nov 20, 2025
@nan-li nan-li force-pushed the ci/add_code_coverage branch from a69c00c to f828623 Compare November 20, 2025 20:50
@nan-li nan-li force-pushed the tests/add_iam_tests branch 4 times, most recently from 1dee76e to be40d3d Compare November 25, 2025 08:57
@nan-li nan-li removed the WIP Work In Progress label Nov 25, 2025
@nan-li nan-li requested a review from a team November 25, 2025 16:32
@nan-li
Copy link
Contributor Author

nan-li commented Nov 25, 2025

Failing tests:
OneSignalNotificationsTests.testClearBadgesWhenAppEntersForeground()
OneSignalNotificationsTests.testDontclearBadgesWhenAppBecomesActive()
Badge tests will be addressed,

Comment on lines 113 to 118
- (void)overrideWebViewContentFinishedLoading:(OSInAppMessageInternal *)message {
if (message) {
[OSMessagingController.sharedInstance messageViewImpressionRequest:message];
// [OSMessagingController.sharedInstance messageViewImpressionRequest:message];
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method does nothing now, can we remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was testing something, didn't mean to commit this one change, I'll undo it.

Comment on lines 1425 to 1444
/// ✅ Migrated to `testPausingIAMs_doesNotCreateMessageQueue`... This test statement below is no longer true as of `5.2.9`.
// - (void)testDisablingIAMs_stillCreatesMessageQueue_butPreventsMessageDisplay {
// let message = [OSInAppMessageTestHelper testMessageJsonWithTriggerPropertyName:OS_DYNAMIC_TRIGGER_KIND_SESSION_TIME withId:@"test_id1" withOperator:OSTriggerOperatorTypeLessThan withValue:@10.0];
// let registrationResponse = [OSInAppMessageTestHelper testRegistrationJsonWithMessages:@[message]];

// // this should prevent message from being shown
// [OneSignal pauseInAppMessages:true];

// // the trigger should immediately evaluate to true and should
// // be shown once the SDK is fully initialized.
// [OneSignalClientOverrider setMockResponseForRequest:NSStringFromClass([OSRequestRegisterUser class]) withResponse:registrationResponse];

// [UnitTestCommonMethods initOneSignal_andThreadWait];

// // Make sure no IAM is showing, but the queue has any IAMs
// XCTAssertFalse(OSMessagingControllerOverrider.isInAppMessageShowing);
// XCTAssertEqual(OSMessagingControllerOverrider.messageDisplayQueue.count, 1);
// }

/// ✅ Migrated to `testPreviewIAMIsDisplayedOnPause`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this code if it as been migrated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that makes sense

@nan-li nan-li requested a review from jkasten2 December 1, 2025 22:02
@nan-li nan-li force-pushed the tests/add_iam_tests branch from 12491ad to 19374af Compare December 1, 2025 22:06
Base automatically changed from ci/add_code_coverage to main December 1, 2025 22:09
Add Swift `IAMIntegrationTests` test file and check for accessibility of `OSMessagingController` from the test file
* Aren't called by anything so no need to be on the interface, these are called by the class itself.
* Without a description, the object will return something like `<OSRequestGetInAppMessages: 0x600001798bc0>`. By adding a predictable description, we can operate on the object.
* Re-usable test helpers and mock functionality
* Include own swiftlint file
* ❗️ After `OneSignalInAppMessagesMocks` was created, I had to then manually "convert to group" or else the CI had build errors about the following:

xcodebuild: error: Unable to read project 'OneSignal.xcodeproj'
Reason: The project ‘OneSignal’ is damaged and cannot be opened. Examine the project file for invalid edits or unresolved source control conflicts.
Exception: didn't find classname for 'isa' key
…alInAppMessages

* This extension on `UIApplication` is only used in the `OneSignalInAppMessages` framework, so there is no need to keep it in the umbrella `OneSignalFramework` framework, which causes errors if only `OneSignalInAppMessages` is tested (without also importing OneSignalFramework).
* Has helper methods for repeating functionality
* Attempt to migrate `testDisablingIAMs_stillCreatesMessageQueue_butPreventsMessageDisplay` to new tests, but behavior has changed. It becomes `testPausingIAMs_doesNotCreateMessageQueue`
* Migrate `testPreviewIAMIsDisplayedOnPause`
* `messageViewImpressionRequest` was removed from the `OSMessagingController` header file, so update `OSMessagingControllerOverrider` to expose it for internal usage.
* Delete old tests that have been migrated
@nan-li nan-li force-pushed the tests/add_iam_tests branch from 19374af to 2267465 Compare December 1, 2025 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants